Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unify Date format #264

Merged
merged 26 commits into from
Oct 24, 2021
Merged

Unify Date format #264

merged 26 commits into from
Oct 24, 2021

Conversation

nicogenz
Copy link
Contributor

Closes #263 .

Changes

  • align date formats

Type

  • New Module
  • Other New Feature
  • Validation Fix
  • Other Bugfix
  • Docs
  • Chore/other

Comments/notes

nicogenz and others added 24 commits August 14, 2021 10:29
@nicogenz
docs: add insiderTransaction example
b8fe684
@nicogenz
docs: add insiderHolders example
fd8abdf
@nicogenz
docs: add netSharePurchaseActivity example
7296069
@nicogenz
docs: add institutionOwnership example
e3f6aa4
@nicogenz
docs: add cashflowStatementHistoryQuarterly example
91f3fc2
@nicogenz
docs: add fundOwnership example
ffd1d7a
@nicogenz
docs: add incomeStatementHistory example
a4240ce
@nicogenz
docs: add incomeStatementHistoryQuarterly example
573f3a2
@nicogenz
docs: add indexTrend example
4e99960
@nicogenz
docs: add industryTrend example
807a6b7
@nicogenz
docs: add majorDirectHolders example
7b8ab03
@nicogenz
docs: add majorHoldersBreakdown example
e68e475
@nicogenz
docs: add quoteType example
3074a81
@nicogenz
docs: add recommendationTrend example
d35de9b
@nicogenz
docs: add sectorTrend example
df8544e
@nicogenz
docs: add fundPerformance example
37dda66
@nicogenz
docs: add fundProfile example
a24e95a
@nicogenz
docs: add missing symbols in example request
69b7bb3
@nicogenz
docs: correct example response
47ea7fb
@nicogenz
docs: correct defaultKeyStatistics example
8432ebf
@nicogenz
docs: align date format
a131e1d
@nicogenz
Merge branch 'gadicc:devel' into devel
cc46217
feat(#263): replace timestamps with date objects
9539b18
docs(#263): update docs according to new date format
9e743d1
@nicogenz
Copy link
Contributor Author

@gadicc The only problem im currently running into are the tests. Some test are failing for example historical.spec.ts without me changing anything to the code. I think this is a problem on my environment. Could you maybe check if they are running?

@advaiyalad
Copy link
Contributor

advaiyalad commented Aug 15, 2021 •
edited
Loading

I believe this would be a breaking change, as you are changing the type of something. You need to change the commit from being feat to feat!.

@gadicc
Copy link
Owner

gadicc commented Aug 15, 2021

Hey @RoXioTD, thanks for your quick work on this!

@PythonCreator27 raises a very valid point (thanks :)). Let me think on this a little. My current thoughts are:

To release in current version:

  • Potentially more of a bugfix than a feature, as this is what we should be doing
  • These modules had no docs with sample output until today, so we're not breaking any API promises
  • Except for TypeScript users. Except on upgrade, tsc will stop them from doing anything dangerous.
  • Still not nice to suddenly have your code stop compiling on an upgrade.
  • But, we still have a "Beta" label plastered everywhere on the README.

To release in next version:

  • Pretty cheap for us to just bump the major release and avoid the above
  • Inconvenient for current users (to be balanced with the inconvenience of errors mentioned above)
  • Users will need to do a major upgrade just to get what is potentially a bugfix.

Let me spend a bit more time thinking about this, and would love if anyone has any further thoughts too.

@advaiyalad
Copy link
Contributor

Ideally, we shouldn't have went to 1.0 straight, but semantic release doesn't allow 0.x, so as of our current state, we should probably put this and a few other breaking changes together, and release a 2.0.

@nicogenz
Copy link
Contributor Author

I agree with @PythonCreator27. Following the semantic versioning this is a breaking change and should be released in a new major release.

@gadicc But, we still have a "Beta" label plastered everywhere on the README. Is this library really in beta still? For me it looks likes it already offers a good amount of functionality and works pretty stable.

@gadicc
Copy link
Owner

gadicc commented Aug 16, 2021

Ok, all, I think we're all on the same page. Going to mark this for a v2 milestone. @RoXioTD will get back to you about the failing tests. And yes, I guess it's about time to drop the beta label... maybe after a smooth v2 release :)

All in all I think the whole point of semantic-release (the automated release toolchain) is to give up getting too worried about doing a major release and make it part of the regular work flow... so i'll try ease into this pattern too 😅

@gadicc gadicc added this to the v2.0 milestone Aug 16, 2021
@nicogenz
Copy link
Contributor Author

@gadicc Any updates on this pull request? Would be cool if we maybe get this change into devel in the near future. 😁

@gadicc
Copy link
Owner

gadicc commented Oct 23, 2021

Hey @RoXioTD, thanks for following up and most especially for your patience! I've been away but am slowly catching up on everything... got through a bunch of PRs and fixes over the last week with an eye to getting ready for this one. I have some time set aside for this tomorrow so let's hope for no unexpected surprises and some good news from me. And thanks again for your patience... I know that after the time and effort of familiarizing yourself with a project and contributing back, it's nice not to have a long wait for that, so... this is high up on my priorities now that I'm catching up on things.

@nicogenz
Copy link
Contributor Author

@gadicc Thank you very much for your quick response and your plan on working soon on this PR. Its perfectly fine for me to wait a little bit more since i dont want to "steal" your freetime to work on an open source project. 😃😃

@gadicc gadicc marked this pull request as ready for review October 24, 2021 09:03
@gadicc gadicc merged commit 4cf1f62 into gadicc:devel Oct 24, 2021
@gadicc
Copy link
Owner

gadicc commented Oct 24, 2021

This looks perfect. I'm going to look at the test issues in devel. If there are any major problems, I'll revert. I'll keep everyone updated here.

gadicc added a commit that referenced this pull request Oct 24, 2021
@gadicc
feat(validation): add support for "date|null" type (#264)
52ea8e4
@gadicc
Copy link
Owner

gadicc commented Oct 24, 2021

I needed to add support for a yahooFinanceType of "date | null" since some of the converted entries used to be "number | null" (and we need to handle the case where Yahoo returns such a field name with a null value). All tests passing now, just missing a bit of coverage, which I'll get to soonest.

gadicc added a commit that referenced this pull request Oct 24, 2021
@gadicc
fix(validation): correctly handle null dates + coverage (#264)
68378d5
@gadicc
Copy link
Owner

gadicc commented Oct 24, 2021

Right I think we're all done! If anyone takes this for a spin please report back. Otherwise this will be in the next release, which will auto-bump the major version.

gadicc pushed a commit that referenced this pull request Oct 27, 2021
@semantic-release-bot
chore(release): 2.0.0 [skip ci]
dcaa688 
# [2.0.0](v1.14.6...v2.0.0) (2021-10-27)

### Bug Fixes

* **search:** allow exchDisp? type ([80236e5](80236e5))
* **validation:** correctly handle null dates + coverage ([#264](#264)) ([68378d5](68378d5))

### Features

* **validation:** add support for "date|null" type ([#264](#264)) ([52ea8e4](52ea8e4))

* feat(quoteSummary)!: unify date format (#264) ([4cf1f62](4cf1f62)), closes [#264](#264) [#263](#263) [#263](#263)

### BREAKING CHANGES

* use `date` instead of `number` for various fields.
The original use of `number` was unintentional.  This commit fixes that.
Unfortunately the type change is a breaking change.

* docs: add insiderTransaction example
* docs: add insiderHolders example
* docs: add netSharePurchaseActivity example
* docs: add institutionOwnership example
* docs: add cashflowStatementHistoryQuarterly example
* docs: add fundOwnership example
* docs: add incomeStatementHistory example
* docs: add incomeStatementHistoryQuarterly example
* docs: add indexTrend example
* docs: add industryTrend example
* docs: add majorDirectHolders example
* docs: add majorHoldersBreakdown example
* docs: add quoteType example
* docs: add recommendationTrend example
* docs: add sectorTrend example
* docs: add fundPerformance example
* docs: add fundProfile example
* docs: add missing symbols in example request
* docs: correct example response
* docs: correct defaultKeyStatistics example
* docs: align date format
@gadicc
Copy link
Owner

gadicc commented Oct 27, 2021

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gadicc gadicc mentioned this pull request Nov 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released on @next
Projects
None yet
Milestone
v2.0
Development

Successfully merging this pull request may close these issues.

quoteSummary dates in additional modules
3 participants
@nicogenz @advaiyalad @gadicc